feat: auto-detect AEM CS Fastly CDN during LLMO onboarding | LLMO-3959#2143
Open
AddyKen-ghost wants to merge 10 commits intomainfrom
Open
feat: auto-detect AEM CS Fastly CDN during LLMO onboarding | LLMO-3959#2143AddyKen-ghost wants to merge 10 commits intomainfrom
AddyKen-ghost wants to merge 10 commits intomainfrom
Conversation
Perform DNS-based detection (CNAME / A-record) inside performLlmoOnboarding, persist the result on the Site model, and include detectedCdn in the HTTP response so the UI can auto-select the correct CDN option. Made-with: Cursor
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…mo | LLMO-3959 Store detectedCdn inside config.llmo instead of as a top-level Site column, eliminating the need for a database migration. The value is persisted in the existing config JSONB column via the shared Config model's new updateLlmoDetectedCdn method. Made-with: Cursor
|
This PR will trigger a minor release when merged. |
Add detectedCdn to ConfigDto.toListJSON whitelist so the CDN detection value is available in list responses, not only in full site detail responses. Made-with: Cursor
…-3959 detectCdnForDomain now distinguishes between: - 'aem-cs-fastly': DNS matched known Fastly signatures - 'other': DNS resolved but did not match (confirmed non-Fastly) - null: DNS lookup failed, result is inconclusive Updated spacecat-shared Joi schema to validate the new values. Updated tests for all three states including mixed-result scenarios. Made-with: Cursor
…-3959 Made-with: Cursor
Made-with: Cursor
…CDN detection ENODATA/ENOTFOUND are authoritative "no records" answers (domain has no CNAME records, or domain doesn't exist). These should fall through to A-record checks instead of being treated as inconclusive DNS failures. Only SERVFAIL/ETIMEOUT-class errors should yield null (inconclusive). Fixes domains like zillow.com (A-records only, no CNAME) returning null instead of 'other'. Made-with: Cursor
Contributor
There was a problem hiding this comment.
Pull request overview
Adds DNS-based CDN detection to LLMO onboarding so the backend can persist the detected CDN on the site config and return it to the UI for auto-selection during onboarding.
Changes:
- Introduce
detectCdnForDomain()(CNAME/A-record based) to identify AEM CS Fastly vs “other” vs inconclusive. - Persist
detectedCdnduringperformLlmoOnboarding()and include it in the LLMO onboarding HTTP response. - Update DTO/OpenAPI and add/extend unit tests to cover the new detection + response/config surface.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/support/slack/actions/onboard-llmo-modal.test.js | Stubs CDN detection to keep Slack onboarding tests deterministic. |
| test/support/cdn-detection.test.js | Adds unit coverage for DNS-based detection outcomes and error handling. |
| test/dto/config.test.js | Ensures detectedCdn is included in ConfigDto.toListJSON(). |
| test/controllers/llmo/llmo-onboarding.test.js | Adds onboarding tests asserting detected CDN is returned and persisted. |
| src/support/cdn-detection.js | Implements DNS-based CDN detection logic (CNAME/A record). |
| src/dto/config.js | Adds llmo.detectedCdn to list DTO output. |
| src/controllers/llmo/llmo.js | Includes detectedCdn in the onboarding HTTP response payload. |
| src/controllers/llmo/llmo-onboarding.js | Calls detection during onboarding and attempts to persist it to site config. |
| package.json | Changes @adobe/spacecat-shared-data-access dependency source/version. |
| package-lock.json | Updates lockfile to match the new spacecat-shared-data-access source/version. |
| docs/openapi/llmo-api.yaml | Documents the new detectedCdn field in the onboarding response schema. |
Made-with: Cursor # Conflicts: # package-lock.json # package.json # src/controllers/llmo/llmo-onboarding.js # src/dto/config.js # test/controllers/llmo/llmo-onboarding.test.js
Satisfy eslint curly rule. Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Perform DNS-based detection (CNAME / A-record) inside performLlmoOnboarding, persist the result on the Site model, and include detectedCdn in the HTTP response so the UI can auto-select the correct CDN option.